fix: sync rows query after error - #3759
Conversation
|
CI is green across the full matrix. This is ready for review when convenient; the change is limited to sending Sync after rows-mode query errors, with regression coverage for rows-mode and normal-query error paths. |
brianc
left a comment
brianc
left a comment
There was a problem hiding this comment.
Hey thanks for this! I'm out of town for another week so I'll be slow on responding but this looks reasonable. I have a question tho before merging this - is there a way to write an integration test that could also replicate the error? I assume issuing an invalid query with rows: 1 on the config could trigger the backend to send an error response and trip the previously offending code path? Unit tests are always nice, but in this case due to the interplay of backend and frontend I'd love to see an integration test covering the behavior. Is that something you'd be willing to include? 🙏
2 participants
Fixes #3707.
When a query uses the
rowsoption,_getRowssendsFlushinstead of pipeliningSync. If PostgreSQL returns anErrorResponse, it ignores subsequent messages until aSyncarrives, so the client can remain permanently stuck waiting forReadyForQuery.This sends
SyncfromhandleErroronly for rows-mode queries, matching the existing rows-mode handling inhandleCommandCompleteandhandleEmptyQuery. Normal queries keep their existing behavior.Tests added for both rows-mode and normal-query error paths.
Validation:
node test/unit/client/query-error-sync-tests.jsmake test-uniteslint packages/pg/lib/query.js packages/pg/test/unit/client/query-error-sync-tests.js